I tested this case on the master : switched off bluetooth, new alerts were not started (new alert with time rule)
Your proposal looks ok anyway, i'll do a few tests and let you know

As for new alert with time rule not firing, well, I believe we have a bug here...
They will not fire no matter how we set the "no alerts on no reception".
In the past the timer used to fire every 5 mintes, no matter what, now it does not, and a new alert time coming in does not trigger it, but this is a different issue.

as far as I can test, time based alert does not fire at all when it's being set or changed while already having a bg value higher than the value set in the alarm (tested with high alert). No matter the status of the missedreadings.

Yes, this is probably a bug in the way that we handle the next alert.
I guess that on the function:
Notifications.ArmTimer();
we should use instead
wakeTime = activeBgAlert.next_alert_at;
wakeTime = min(activeBgAlert.next_alert_at, [time of next alert])

Actually, if there are alerts that are time dependent, we should call the SetTimer() in any case.
This is probably another bug that we have here.
Once this bug will be fixed we can see my fix working...

We would have to do two things:

1st: like you proposed wakeTime = min(activeBgAlert.next_alert_at, "start-time of next alertType")

2nd: call the Notifications Service/Intent every time an alarm gets
saved. Saving an alarm (new or modified) can change the value of
"start-time of next alertType". This would also handle the case where an
alarm gets changed/added that should alarm NOW as the last value already
was "out of Range".

I don't know if we do the 2nd already. But I think not.

On 27/11/15 09:18, tzachi-dar wrote:

Yes, this is probably a bug in the way that we handle the next alert.
I guess that on the function:
Notifications.ArmTimer();
we should use instead
wakeTime = activeBgAlert.next_alert_at;
wakeTime = min(activeBgAlert.next_alert_at, [time of next alert])

Actually, if there are alerts that are time dependent, we should call
the SetTimer() in any case.
This is probably another bug that we have here.
Once this bug will be fixed we can see my fix working...

—
Reply to this email directly or view it on GitHub
#184 (comment).

FREE 3D EARTH SCREENSAVER - Watch the Earth right on your desktop!
Check it out at http://www.inbox.com/earth

The first version of the code, used to have a timer that worked every minute. This solved all this problems with very small code.
Obviously, this takes too much power now-days.
Now we have to look for all this places ourselves and get them right... complicated code...

A lot has changed in the last 2 month. Is this PR still valid?

I'll look at it this week. Might be that the new changes actually make it more ready for submission.